Add doc_url field to match its recent addition in pprof proto.#588
Add doc_url field to match its recent addition in pprof proto.#588aalexand wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
github.com/google/pprof/pull/888 added this field in pprof proto, this change adds it to the OTel profiling proto to match the change.
|
@florianl @jhalliday FYI |
| // | ||
| // The URL may be missing if the profile was generated by older code or code | ||
| // that did not bother to supply a link. | ||
| int64 doc_url = 19; // Index into string table. |
There was a problem hiding this comment.
With ScopeProfiles.schema_url the URL that describes the provided profile exists already. Where do you see the differences between the two, so that this additional field is required?
There was a problem hiding this comment.
schema_url points to https://opentelemetry.io/docs/specs/otel/schemas/#schema-url file which is not user docs. The doc_url here is for user-level documentation. I would also expect that the schema URL would be the same for many profiles as it corresponds to the version of the data schema or something like that rather than the nature of the data (CPU profile vs heap profile vs contention profile).
There was a problem hiding this comment.
Since this should be an optional field, what do you think about using an attribute?
On a similar note, we had a discussion in the last SIG about the boolean fields in Mapping and the consensus was to drop them from the proto and turn them into attributes.
There was a problem hiding this comment.
I think the doc_url can be an attribute. I'll change this.
For the has_* fields I'm less sure. I added an agenda topic to the SIG notes doc with some thoughts on this.
Co-authored-by: Florian Lehner <florianl@users.noreply.github.com>
… ProfileContainer. (#590) This change removes the now unnecessary split of the profiling format between two files, combining them into one as with other protocols. In the process, the likewise unnecessary ProfileContainer message type is removed and its fields merged into Profile, saving one level of indirection. Note this as discussed on slack otel-profiles, this is effectively a minimal subset of the now defunct #587, remaining parts of which will be resubmitted in subsequent PRs. This PR effectively blocks any concurrent changes to pprofextended.proto e.g. #588 as git won't track such changes across the file 'merge'.
|
We decided to make this an attribute instead. |
5 participants
google/pprof/pull/888 added this field in pprof proto, this change adds it to the OTel profiling proto to match the change.